Skip to content

Pipeline Performance Optimization: Make Static Analysis Run Parallelly with Build #5364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
May 8, 2025

Conversation

MuyuanMS
Copy link
Contributor

@MuyuanMS MuyuanMS commented Apr 25, 2025

This PR shortens build time by around 50 minutes for foundation repo, while only increase absolute build time by 25 minutes.

Below are running results for Foundation PR Build:

  • Before Change

    • Total Build Time: 2h 26m
    • Absolute Build Time (Sum of All jobs): 582 min
      image
  • After Change

    • Total Build Time: 1h 27m (-40%)
    • Absolute Build Time (Sum of All jobs): 607 min (+4.3%)
      image

Currently largest bottleneck building foundation is static analysis, which can be run independently but currently blocks dependent stage from running. After this change, now during build, two build stage will start in parallel - one without static analysis to allow dependent steps like Test or Pack to run, the other with static analysis


A microsoft employee must use /azp run to validate using the pipelines below.

WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.

For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Haonan Tang (from Dev Box) and others added 2 commits April 25, 2025 16:56
@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS MuyuanMS changed the base branch from main to user/haonanttt/testStageSeparation April 28, 2025 05:33
@MuyuanMS
Copy link
Contributor Author

/azp run

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

I see 2 basic options: 1) continue to only run PREfast on x64fre only, that means we skip the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages, which hopefully will allow the pipeline to finish even sooner, 2) adjust the code cited above to enable running PREfast for x86 and arm64 in the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages.

@alexlamtest I have updated current PR with option 1. Below are some of the results while making the decision:

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yeelam-gordon
Copy link

I see 2 basic options: 1) continue to only run PREfast on x64fre only, that means we skip the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages, which hopefully will allow the pipeline to finish even sooner, 2) adjust the code cited above to enable running PREfast for x86 and arm64 in the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages.

@alexlamtest I have updated current PR with option 1. Below are some of the results while making the decision:

@alexlamtest, I will suggest let Muyuan to checkin the one only improve performance first, which the result is promising and without increase much build agent cost (i.e. 4.3% only)
And if we believe it is necessary to run statistic analysis against all the build architect, it should be an easy change later on.

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 8, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alexlamtest
Copy link
Contributor

alexlamtest commented May 8, 2025

_>how I can confirm whether PREfast results are the same for x86, x64, and arm64?
You can use a OneBranch pipeline: https://microsoft.visualstudio.com/ProjectReunion/_build/results?buildId=121978283&view=logs&j=096205eb-4599-56da-affa-13246cc49eac. If you look at the end of each build job, there is a "Guardian: Post Analysis" task there. PREfast results can be found in there (non-OneBranch pipelines, such as the validation pipeline run for this PR, does not have that Guardian task). But note that for the pipeline run I cited above, even for the x64 build, there is no PREfast results shown in the Guardian task. That's simply because the cited pipeline run's parameter has runStaticAnalysis=false. If you set that parameter to true when you manually trigger the pipeline to run, or you use yml code changes to set the parameter to true, then you should be able to find PREfast results there, at least in the x64 cause. You would want to enable PREFast to run also for x86 and amd64. Then you should get PREfast results for all 3 architectures and you can confirm that their results are very similar. From my experience many months ago, the difference was 5% or less, and x64 usually found the most PREfast issues. Hope this helps!

Yes, I agree with Gordon that we can submit the changes for option #1. The latest perf numbers look inline with my expectation. The changes LGTM, I already approved the PR. Thank you for this great work!

@MuyuanMS MuyuanMS merged commit 2d8deff into main May 8, 2025
34 checks passed
@MuyuanMS MuyuanMS deleted the user/muyuanli/parallelAnalysis branch May 8, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants